Skip to content

Add typos checks#36874

Closed
bircni wants to merge 4 commits intogo-gitea:mainfrom
bircni:feature/typos-checks
Closed

Add typos checks#36874
bircni wants to merge 4 commits intogo-gitea:mainfrom
bircni:feature/typos-checks

Conversation

@bircni
Copy link
Copy Markdown
Member

@bircni bircni commented Mar 9, 2026

As requested by @lunny in #36783 I added a typos check

We currently cannot solve all old typos - please tell me if there are some which I would be allowd to fix
For now I only tried to fix some non-breaking ones

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 9, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/docs modifies/internal labels Mar 9, 2026
Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs more sane way of ignoring base64 strings.

Comment on lines +55 to +57
helo = "helo"
Helo = "Helo"
HELO = "HELO"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one of them should be enough

cpy = "cpy"
odf = "odf"
# SunOS SMF framework property name
startd = "startd"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's ignored though?

Comment on lines +152 to +158
[default.extend-identifiers]
# util.Iif is ternary helper; cannot change: public API
Iif = "Iif"
iif = "iif"
# determineSHAforPR function name; cannot change: public API
SHAforPR = "SHAforPR"
Afor = "Afor"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead rules?

Comment on lines +138 to +140
# ARIA attribute aria-activedescendant; HTML standard
actived = "actived"
Actived = "Actived"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad comment, all the mentions I see refer to dropped column in database.

Comment on lines +58 to +62
# File extension / format name (e.g. Clojure edn, Makefile .mak)
edn = "edn"
mak = "mak"
cpy = "cpy"
odf = "odf"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# File extension / format name (e.g. Clojure edn, Makefile .mak)
edn = "edn"
mak = "mak"
cpy = "cpy"
odf = "odf"

Comment on lines +117 to +118
# File extension / icon name in material-icon-rules.json
stap = "stap"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# File extension / icon name in material-icon-rules.json
stap = "stap"
# File extension / icon name in material-icon-rules.json
stap = "stap"
edn = "edn"
mak = "mak"
cpy = "cpy"
rcall = "rcall"
caf = "caf"

Comment on lines +101 to +104
caf = "caf"
claus = "claus"
Claus = "Claus"
rcall = "rcall"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
caf = "caf"
claus = "claus"
Claus = "Claus"
rcall = "rcall"

Comment on lines +76 to +100
# Substring in base64 or test data (false positive)
Ot = "Ot"
nd = "nd"
Nd = "Nd"
ND = "ND"
iy = "iy"
Iy = "Iy"
ba = "ba"
BA = "BA"
Pn = "Pn"
Iz = "Iz"
ue = "ue"
Ue = "Ue"
Mis = "Mis"
Ser = "Ser"
Yto = "Yto"
fo = "fo"
ist = "ist"
Ein = "Ein"
noo = "noo"
oder = "oder"
ono = "ono"
ags = "ags"
alle = "alle"
ALLWAYS = "ALLWAYS"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while typos has indeed problem with base64 this is not a good approach.
It's a huge bag of items which at times are base64 garbage, are test data or emoji data while it skips a real issue of "Mis" in "Mis-configured".
ALLWAYS is should not be labeled as test data since it's an old config key.

# Migration function RemoveLabelUneededCols; cannot rename
Uneeded = "Uneeded"
# Emoji shortcode "womens" (women's room) in emoji_data.go
womens = "womens"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
womens = "womens"
womens = "womens"
claus = "claus"

Comment on lines +47 to +53
# TypoScript is a CMS project name, not a typo of TypeScript
TypoScript = "TypoScript"
# Meilisearch ranking rule name
typo = "typo"
# TYPO3 CMS project name
TYPO3 = "TYPO3"
Typo3 = "Typo3"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead rules?

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 9, 2026
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Mar 10, 2026

We already have misspell as a spellchecker and one thing I like about it is it has a near-zero rate of false-positives. I tried integrating other spellcheckers before but always gave up on their enormous rate of false-positives.

I think we should not have to babysit a spell checker configuration. If this one works well, I'm not against switching to it, but it needs to be absolutely robust.

unparseable = "unparseable"
# i18n key used in all locale files; cannot change key
enterred = "enterred"
# Substring in base64 or test data (false positive)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Don't do this.

- name: Typos check
uses: crate-ci/typos@v1.44.0
with:
config: _typos.toml
Copy link
Copy Markdown
Member

@silverwind silverwind Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action-only checks please. Run it during make lint-spell and remove this step. That way it can be ran locally which is very important.

You probably need to add a cargo.toml to pin this tool dependency, similar to what we already do for python using pyproject.toml.

Also if possible integrate it into make lint-spell-fix to automatically fix all errors.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action-only checks please

lint-typos was added in the PR so it's runnable locally. Not sure why it's a separate rule and it doesn't cover installation (which I don't mind) but it does exist.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I missed that, was only seeing this.

@bircni
Copy link
Copy Markdown
Member Author

bircni commented Mar 10, 2026

Closing this as typos-cli does not cover all features we want it to have - would be a nice thing to have but not possible in the condition as typos-cli currently is

@bircni bircni closed this Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged modifies/docs modifies/go Pull requests that update Go code modifies/internal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants